Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

milindcq/gh-84: pipelineID resolution fix for multi/mixed pipeline stages #83

Merged
merged 2 commits into from
Aug 10, 2023
Merged

milindcq/gh-84: pipelineID resolution fix for multi/mixed pipeline stages #83

merged 2 commits into from
Aug 10, 2023

Conversation

milindcq
Copy link
Contributor

@milindcq milindcq commented Aug 8, 2023

Overview

Github Issue: #84

Summary (required always)

While rendering a nested pipeline If one of the pipeline stage is a nested pipeline and another refers to an existing pipeline. The spinnaker pipeline id for the existing pipeline stage is not resolved. This is due to faulty check that skips the id resolution due to this check (

if !hasChildPipelines && mapContainsKey(innerStage, "application") && mapContainsKey(innerStage, "pipeline") && reflect.TypeOf(innerStage["pipeline"]).Kind() == reflect.String {
))

Notes

Unit-tests result:

unit-test-results.txt

Before (failed pipeline id resolution):
Screenshot 2023-08-10 at 10 25 24 AM

After the PR the pipeline ID was resolved successfully:
Screenshot 2023-08-10 at 10 30 10 AM

Checklist

  • My pull request title follows the format <username>/<gh-issue-#number>:<short-description>
  • My code passes existing unit tests
  • My code follows the code style set for the project
  • I have added at least one reviewer for this PR

@milindcq milindcq requested a review from a team as a code owner August 8, 2023 21:20
@github-actions
Copy link

github-actions bot commented Aug 8, 2023

CLA Assistant Lite bot: Thank you for your submission, we really appreciate it. We ask that you sign our Contributor License Agreement before we can accept your contribution.

If you are contributing on behalf of your employer you must fill out our Corporate Contributor License Agreement which can be found here.
If you are contributing on behalf of yourself you must agree to our Individual Contributor License Agreement by reviewing this document and signing it or by replying below with a comment containing the following text:


I have read the CLA Document and I hereby sign the CLA


Milind Mistry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@milindcq milindcq changed the title If one of the pipeline stages in a nested pipeline and the other(s) refers to an existing pipeline. The shore does not resolve the pipelineID (from spinnaker) of the existing pipelines. If one of the pipeline stages in a nested pipeline and the other(s) refers to an existing pipeline. The shore does not resolve the pipelineID (from spinnaker) of an existing pipelines. Aug 8, 2023
@dkirillov
Copy link
Collaborator

@milindcq Thank you for eagerly trying to contribute!

While the change itself is small and seemingly good - the more logistical aspect of the PR isn't.

Please read the CONTRIBUTE.md doc for how to contribute.

The PR needs an issue, so please open one and describe the problem in it - and link it in the description of the PR and the title (as in the PR's checklist).

Please also show that the unit tests are passing and that you have tried using the change against the problem that you've encountered (or a similar scenario) as well as any other scenarios that help prove that the change works and does not break anything.

Because this is open source - everyone is setting an example and precedent for one another, and will be held accountably equally - to ensure quality is maintained.

@dkirillov dkirillov changed the title If one of the pipeline stages in a nested pipeline and the other(s) refers to an existing pipeline. The shore does not resolve the pipelineID (from spinnaker) of an existing pipelines. milindcq/gh-84: pipelineID resolution fix for multi/mixed pipeline stages Aug 10, 2023
@dkirillov dkirillov added bug Something isn't working save Related to "shore save" Backend-Spinnaker The issue is related to the built-in Spinnaker Backend labels Aug 10, 2023
@milindcq
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@milindcq
Copy link
Contributor Author

recheck

@milindcq milindcq closed this Aug 10, 2023
@milindcq milindcq reopened this Aug 10, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2023
@milindcq milindcq marked this pull request as draft August 10, 2023 18:08
@milindcq milindcq marked this pull request as ready for review August 10, 2023 18:08
@Autodesk Autodesk unlocked this conversation Aug 10, 2023
Copy link
Collaborator

@dkirillov dkirillov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the bug fix!

@dkirillov dkirillov merged commit 6d5ad24 into Autodesk:main Aug 10, 2023
12 of 13 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Backend-Spinnaker The issue is related to the built-in Spinnaker Backend bug Something isn't working save Related to "shore save"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants